Closed
Bug 1375596
Opened 8 years ago
Closed 8 years ago
SEGV near null in [@ mozilla::StyleAnimationValue::AddWeighted]
Categories
(Core :: SVG, defect, P1)
Core
SVG
Tracking
()
RESOLVED
FIXED
mozilla56
Tracking | Status | |
---|---|---|
firefox-esr52 | --- | unaffected |
firefox54 | --- | unaffected |
firefox55 | --- | fixed |
firefox56 | --- | fixed |
People
(Reporter: tsmith, Assigned: birtles)
References
(Blocks 1 open bug)
Details
(Keywords: crash, regression, testcase)
Attachments
(2 files)
102 bytes,
text/html
|
Details | |
59 bytes,
text/x-review-board-request
|
hiro
:
review+
jcristau
:
approval-mozilla-beta+
|
Details |
==14379==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000020 (pc 0x7f832a43546b bp 0x7fffc3779390 sp 0x7fffc3778fe0 T0)
==14379==The signal is caused by a READ memory access.
==14379==Hint: address points to the zero page.
#0 0x7f832a43546a in GetUnit src/obj-firefox/dist/include/mozilla/StyleAnimationValue.h:356:12
#1 0x7f832a43546a in mozilla::StyleAnimationValue::AddWeighted(nsCSSPropertyID, double, mozilla::StyleAnimationValue const&, double, mozilla::StyleAnimationValue const&, mozilla::StyleAnimationValue&) src/layout/style/StyleAnimationValue.cpp:2869
#2 0x7f8329b271c7 in Add src/obj-firefox/dist/include/mozilla/StyleAnimationValue.h:62:12
#3 0x7f8329b271c7 in AddOrAccumulate(nsSMILValue&, nsSMILValue const&, mozilla::dom::CompositeOperation, unsigned long) src/dom/smil/nsSMILCSSValueType.cpp:413
#4 0x7f8329b2664b in nsSMILCSSValueType::SandwichAdd(nsSMILValue&, nsSMILValue const&) const src/dom/smil/nsSMILCSSValueType.cpp:422:10
#5 0x7f8329b1dbc7 in nsSMILAnimationFunction::ComposeResult(nsISMILAttr const&, nsSMILValue&) src/dom/smil/nsSMILAnimationFunction.cpp:271:22
#6 0x7f8329b1a7ce in nsSMILCompositor::ComposeAttribute(bool&) src/dom/smil/nsSMILCompositor.cpp:108:29
#7 0x7f8329b17e6a in nsSMILAnimationController::DoSample(bool) src/dom/smil/nsSMILAnimationController.cpp:455:17
#8 0x7f832a786bbb in Resample src/obj-firefox/dist/include/nsSMILAnimationController.h:74:21
#9 0x7f832a786bbb in FlushResampleRequests src/obj-firefox/dist/include/nsSMILAnimationController.h:90
#10 0x7f832a786bbb in mozilla::PresShell::DoFlushPendingNotifications(mozilla::ChangesToFlush) src/layout/base/PresShell.cpp:4165
#11 0x7f832689e1f1 in FlushPendingNotifications src/obj-firefox/dist/include/nsIPresShell.h:560:5
#12 0x7f832689e1f1 in nsDocument::FlushPendingNotifications(mozilla::FlushType) src/dom/base/nsDocument.cpp:8076
#13 0x7f8325842fdb in nsDocLoader::DocLoaderIsEmpty(bool) src/uriloader/base/nsDocLoader.cpp:702:14
#14 0x7f8325845302 in nsDocLoader::OnStopRequest(nsIRequest*, nsISupports*, nsresult) src/uriloader/base/nsDocLoader.cpp:631:5
#15 0x7f832584602c in non-virtual thunk to nsDocLoader::OnStopRequest(nsIRequest*, nsISupports*, nsresult) src/uriloader/base/nsDocLoader.cpp:487:14
#16 0x7f8323f95753 in mozilla::net::nsLoadGroup::RemoveRequest(nsIRequest*, nsISupports*, nsresult) src/netwerk/base/nsLoadGroup.cpp:629:28
#17 0x7f83268a505b in nsDocument::DoUnblockOnload() src/dom/base/nsDocument.cpp:8929:18
#18 0x7f83268a4c43 in nsDocument::UnblockOnload(bool) src/dom/base/nsDocument.cpp:8855:9
#19 0x7f832687c56d in nsDocument::DispatchContentLoadedEvents() src/dom/base/nsDocument.cpp:5348:3
#20 0x7f8326947ed2 in applyImpl<nsDocument, void (nsDocument::*)()> src/obj-firefox/dist/include/nsThreadUtils.h:1084:12
#21 0x7f8326947ed2 in apply<nsDocument, void (nsDocument::*)()> src/obj-firefox/dist/include/nsThreadUtils.h:1090
#22 0x7f8326947ed2 in mozilla::detail::RunnableMethodImpl<nsDocument*, void (nsDocument::*)(), true, (mozilla::RunnableKind)0>::Run() src/obj-firefox/dist/include/nsThreadUtils.h:1133
#23 0x7f8323dfb798 in nsThread::ProcessNextEvent(bool, bool*) src/xpcom/threads/nsThread.cpp:1416:14
#24 0x7f8323e08678 in NS_ProcessNextEvent(nsIThread*, bool) src/xpcom/threads/nsThreadUtils.cpp:472:10
#25 0x7f8324bb5221 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:96:21
#26 0x7f8324b133a0 in RunInternal src/ipc/chromium/src/base/message_loop.cc:318:10
#27 0x7f8324b133a0 in RunHandler src/ipc/chromium/src/base/message_loop.cc:311
#28 0x7f8324b133a0 in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:291
#29 0x7f832a0699ef in nsBaseAppShell::Run() src/widget/nsBaseAppShell.cpp:156:27
#30 0x7f832e0e6431 in nsAppStartup::Run() src/toolkit/components/startup/nsAppStartup.cpp:283:30
#31 0x7f832e2b6674 in XREMain::XRE_mainRun() src/toolkit/xre/nsAppRunner.cpp:4569:22
#32 0x7f832e2b81e0 in XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) src/toolkit/xre/nsAppRunner.cpp:4749:8
#33 0x7f832e2b9531 in XRE_main(int, char**, mozilla::BootstrapConfig const&) src/toolkit/xre/nsAppRunner.cpp:4844:21
#34 0x4eb613 in do_main src/browser/app/nsBrowserApp.cpp:237:22
#35 0x4eb613 in main src/browser/app/nsBrowserApp.cpp:310
#36 0x7f834099182f in __libc_start_main /build/glibc-bfm8X4/glibc-2.23/csu/../csu/libc-start.c:291
#37 0x41d168 in _start (browsers/m-c-1497454412-asan-opt/firefox+0x41d168)
AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV src/obj-firefox/dist/include/mozilla/StyleAnimationValue.h:356:12 in GetUnit
Flags: in-testsuite?
Assignee | ||
Comment 2•8 years ago
|
||
Likely a recent regression from all the refactoring we've done of that code for stylo.
Assignee | ||
Updated•8 years ago
|
Priority: -- → P1
Assignee | ||
Comment 3•8 years ago
|
||
I'm still waiting for a build to finish to debug this but the following changeset from bug 1358966 looks suspicious since we dereference valueToAddWrapper without any guarantee that it is not null (prior to that changeset we dereferenced valueToAdd which we do null-check FinalizeStyleAnimationValues):
https://hg.mozilla.org/mozilla-central/rev/4d87f2bf4b10369af0dd83a2ef962a23299ee8d9
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → bbirtles
Status: NEW → ASSIGNED
Assignee | ||
Updated•8 years ago
|
Blocks: 1358966
status-firefox54:
--- → unaffected
status-firefox55:
--- → affected
status-firefox56:
--- → affected
Keywords: regression
Comment hidden (mozreview-request) |
Comment 5•8 years ago
|
||
mozreview-review |
Comment on attachment 8887358 [details]
Bug 1375596 - Use valueToAdd in AddAccumulateOrValue, not valueToAddWrapper;
https://reviewboard.mozilla.org/r/158190/#review163462
Attachment #8887358 -
Flags: review?(hikezoe) → review+
Pushed by bbirtles@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/86878427cd44
Use valueToAdd in AddAccumulateOrValue, not valueToAddWrapper; r=hiro
Comment 7•8 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Flags: in-testsuite? → in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Comment 8•8 years ago
|
||
Please request Beta approval on this when you get a chance.
status-firefox-esr52:
--- → unaffected
Flags: needinfo?(bbirtles)
Assignee | ||
Comment 9•8 years ago
|
||
Comment on attachment 8887358 [details]
Bug 1375596 - Use valueToAdd in AddAccumulateOrValue, not valueToAddWrapper;
Approval Request Comment
[Feature/Bug causing the regression]: bug 1358966
[User impact if declined]: Content process crash when opening offending SVG content
[Is this code covered by automated tests?]: Yes
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: No (has automated test)
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: No
[Why is the change risky/not risky?]: Simply makes us dereference a pointer that is known to be non-null as opposed to one that is not guaranteed to be non-null
[String changes made/needed]: None
Flags: needinfo?(bbirtles)
Attachment #8887358 -
Flags: approval-mozilla-beta?
Comment 10•8 years ago
|
||
Comment on attachment 8887358 [details]
Bug 1375596 - Use valueToAdd in AddAccumulateOrValue, not valueToAddWrapper;
fix nullptr deref crash, beta55+
Attachment #8887358 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 11•8 years ago
|
||
bugherder uplift |
Comment 12•8 years ago
|
||
(In reply to Brian Birtles (:birtles) from comment #9)
> [Is this code covered by automated tests?]: Yes
> [Has the fix been verified in Nightly?]: Yes
> [Needs manual test from QE? If yes, steps to reproduce]: No (has automated
> test)
Setting qe-verify- based on Brian's assessment on manual testing needs and the fact that this fix has automated coverage.
Flags: qe-verify-
You need to log in
before you can comment on or make changes to this bug.
Description
•